-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sierra: Add translation prefix support for ILS messages. #3140
Sierra: Add translation prefix support for ILS messages. #3140
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, apart from a minor complaint about the comments.
config/vufind/SierraRest.ini
Outdated
; not used, but a prefix may be used to distinguish the codes from any other | ||
; translations (or other libraries). To use a simple prefix in the default text | ||
; domain: | ||
; translationPrefix = "sierra_" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments are a little confusing here; you have text explaining the first example, but no text explaining the second example... and then the "actual" commented-out setting is identical to the second example. How would you feel about eliminating the two example lines, and changing the last sentence to something like the following?
You can use a simple prefix in the default domain (e.g. "sierra_") or a custom text domain (e.g. "ILSMessages::").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just copied from Alma.ini, but I'll revise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be better now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @EreMaijala!
Very much alike the one in Alma.